Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch for bug #61660 #40

Closed
wants to merge 2 commits into from
Closed

Patch for bug #61660 #40

wants to merge 2 commits into from

Conversation

krtek4
Copy link

@krtek4 krtek4 commented Apr 8, 2012

This is a patch for : https://bugs.php.net/bug.php?id=61660

I'm the one who reported the bug and I tested the patch in the particular case which led to his discovery.

Their can still be a problem with bin2hex. The reverse function is not aware of left padded values and thus return '01234' for example which can lead to some problems like stated by laruence in the bug comments.

I think bin2hex should also be patched to remove any leading 0s in the binary representation before converting it, but this is out of the scope of my patch and needs discussion.

When the hexadecimal string has an odd length, the resulting binary data must be left padded with 0s in order to be aligned on 8 bits.
To avoid extraneous if test on each iteration, we distinguish the special case by pointing the source data iterator the the NUL bytes at the end which we can then detect and act accordingly.
@krtek4
Copy link
Author

krtek4 commented Apr 8, 2012

I changed the comment style and reverted the encoding problems.

BTW the encoding problem was coming from the github inline editor since I did my first patch through the web interface.

@krtek4
Copy link
Author

krtek4 commented Apr 8, 2012

I'll redo the pull request cleanly.

@krtek4 krtek4 closed this Apr 8, 2012
nikic pushed a commit to nikic/php-src that referenced this pull request Jan 20, 2013
php-pulls pushed a commit that referenced this pull request Dec 20, 2013
In english, no horizontal space before “!”
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant